Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add layout file when collect ssr loader #11666

Merged
merged 3 commits into from
Sep 26, 2023
Merged

Conversation

ppppp-x-x
Copy link
Contributor

Background

解决在收集 ssr 路由相关信息时遗漏 layout 文件的问题
目前在编译时收集路由 serverLoader 时只包含了页面文件,layout 文件也需要支持配置 serverLoader、clientLoader

Solution

  • 改变收集数据的位置,调整到 add layout 之后执行

@vercel
Copy link

vercel bot commented Sep 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
umi ⬜️ Ignored (Inspect) Visit Preview Sep 25, 2023 4:57pm

Copy link
Contributor

@fz6m fz6m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自己排查下 CI 吧,这个时候 layout 可能还没生成出来。

@ppppp-x-x
Copy link
Contributor Author

自己排查下 CI 吧,这个时候 layout 可能还没生成出来。

好的,我排查一下 CI~但是“这个时候 layout 可能还没生成出来”请问下是什么意思呢,我看这里 layout 已经 add 进去了

@fz6m
Copy link
Contributor

fz6m commented Sep 22, 2023

我看有的报错是检查文件存在性失败了,目测有的 layout 是由插件生成的临时文件作为 layout ,这个 collect 的时候可能 layout 文件可能还没生成出来。

@ppppp-x-x
Copy link
Contributor Author

我看有的报错是检查文件存在性失败了,目测有的 layout 是由插件生成的临时文件作为 layout ,这个 collect 的时候可能 layout 文件可能还没生成出来。

我排查了下,这里应该是因为我调整了 routes 中的“layout”,为它增加了 hasServerLoader | clientLoader | routeProps 导致 jest snapshot 与之前生成的不一致导致的
如果要 layout 文件也支持 export serverLoader |clientLoader 的话,这里是否可以给 routes => layout 添加这些属性呢~如果可以的话,需要更新下测试快照

@fz6m
Copy link
Contributor

fz6m commented Sep 23, 2023

可以,改了看一下。

@fz6m
Copy link
Contributor

fz6m commented Sep 25, 2023

还是有问题,再检查下 CI

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (db1d0b8) 28.99% compared to head (4ad9bb2) 29.00%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11666   +/-   ##
=======================================
  Coverage   28.99%   29.00%           
=======================================
  Files         485      485           
  Lines       14728    14729    +1     
  Branches     3486     3487    +1     
=======================================
+ Hits         4271     4272    +1     
  Misses       9700     9700           
  Partials      757      757           
Files Coverage Δ
...ackages/preset-umi/src/features/tmpFiles/routes.ts 57.93% <100.00%> (+0.33%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sorrycc
Copy link
Member

sorrycc commented Sep 25, 2023

ci 还是有问题,我看了下第一个是「ENOENT: no such file or directory, open '/home/runner/work/umi/umi/examples/max/.umi-production/plugin-layout/Layout.tsx'」,得排查下原因看。可能的原因是,可能有些虚拟文件已经有 __content 等信息了,不需要读文件系统。

@ppppp-x-x
Copy link
Contributor Author

ci 还是有问题,我看了下第一个是「ENOENT: no such file or directory, open '/home/runner/work/umi/umi/examples/max/.umi-production/plugin-layout/Layout.tsx'」,得排查下原因看。可能的原因是,可能有些虚拟文件已经有 __content 等信息了,不需要读文件系统。

CI 的问题是在虚拟 layout 文件在 getRoutes 的时候没有获取到,我在这里判断了是否是 layout 文件,如果是的话不增加额外的属性,这样的话应该和之前生成的数据也是一致的

@ppppp-x-x
Copy link
Contributor Author

还是有问题,再检查下 CI

好的~

@sorrycc sorrycc merged commit b4b3c6e into umijs:master Sep 26, 2023
@ppppp-x-x ppppp-x-x deleted the fix/px_srr branch November 30, 2023 09:27
@ppppp-x-x ppppp-x-x restored the fix/px_srr branch November 30, 2023 09:28
@ppppp-x-x ppppp-x-x deleted the fix/px_srr branch November 30, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants